I am not sure if the defaults are actually preserved all the way down... it might just be the top-level object. would have to check.

yes, they seem to be preserved - at least in the sense of being correctly displayed in the settings editor; how else can I specify them? properties → completion and then default?

This signature is already way too complicated. A more common pattern would be to have a ILanguageServerManager.IOptions which defined all these pieces, and didn't conflate the variable scopes (public/private) in the constructor. Putting this in a (new) tokens.ts will help downstreams understand what the language server manager can provide to them.

The main advantage here is that it is easier to add a property to an unordered set of named arguments rather than getting the perfect ordering of many disparate types.

I get that there are a few advantages of IOptions pattern, but it does not seem greatly useful for a function which gets called once ever (and with arguments that we do not control). Besides, having IOptions here would just move the problem of relying on a specific ordering of arguments to the activate() function (which already led to a hard-to-debug issue - explicit typing with a list of types is based on that lesson).

advantages, off the top of my head:

• easier to document
• easier to extend
• better completion
• more like core

activate is needfully the special case, at the application level, so that things can be replaced.

i see what you're doing here, and yes, relying on so many things is onerous. but this seems like a very roundabout way to leverage the type system... and basically guarantees that the signature, and therefore the API, will break the next time this changes. Sure, you're not returning this superobject yet, but this would all but need to be the public API.

my experience has been to minimize the number of external APIs that are actually called inside the manager class, and favor connecting to top-level signals in the extension.ts. this centralizes the potential API surface to the best

1. not returning because activate has void signature
2. as long as the requires and as [] stay in sync this will not break (ideally there would be a const TOKENS = [...] on type TOKENS = [...] defined in a single line, but I could not get it working)
3. yes the public API will change the next time we add a token dependency, but is this a bad thing? LSPExtension is only meant to be used in the activate function (can document this). I see that I export it but maybe if I exported an interface with the things that are meant to be passed down, then LSPExtension would no longer be public. On the other hand, I do not feel convenient having the extra interface if I can just have an extension object. The extension class just seems so natural to have (and it to contain app and settings references)

Quick note: 1 no longer the case in the refactor, this was my limited understanding.

same thing here: when we start changing constructors, we should just go ahead and adopt an IOptions someplace so the options can grow more organically over time. The closer we can start making <ActivityName>WidgetAdapter invocation to be more regular, I think the happier we'll be...

we should probably collect all of our external command references someplace...

here, you could reus the T from above, and always set to a Partial<T>: fewer magic strings, and no looping (outside of here) to set multiple values.

I do not understand. Could you throw me some example, plese?

i'll see if i can dig something up. Basically, if you know T, you can ask for setting: keyof T, and then the value can be inferred by type inference. I think lumino has some stuff like that.

if possible, we should try to keep references to the application itself to a minimum...

indeed, many of these things could just become dumb getters that wrap this.extension.X (or this.language_server_manager.X)

this seems like sort of a weird place to land this...

see https://github.com/krassowski/jupyterlab-lsp/pull/315/files#diff-768f6aea86149c0aa51e05111ea0968cR160. I don't like it either but seems the best thing to maintain separation of lab-specific and codemirror-specific code without major refactoring. I mean it should probably go like Feature = LabFeature + CMFeature + Settings (which are relevant to both) - I can work on it in a subsequent PR.